produce userdata for VSCode to consume by Eskibear · Pull Request #40 · microsoft/java-debug

Eskibear · 2017-09-20T09:14:24Z

In VSCode, use following method to get userdata object.

function fetchUserData() {
    return executeJavaLanguageServerCommand(commands.JAVA_FETCH_USER_DATA);
}

msftclas · 2017-09-20T09:14:28Z

@Eskibear,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

Eskibear · 2017-09-20T09:26:22Z

Moreover, I need an AI key for VSCode to send user data. @akaroml

yaohaizh · 2017-09-20T12:15:37Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java

 public class ProtocolServer {
    private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME);

+    private static final Logger loggerUserdata = Logger.getLogger(Configuration.USERDATA_LOGGER_NAME);


Naming: userDataLogger to make it consistent and readable

yaohaizh · 2017-09-20T12:18:28Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java


    private IDebugAdapter debugAdapter;

+    /* userdataMap Schema:


Use javadoc style comments

andxu · 2017-09-22T08:51:07Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+                for (byte b : hashBytes) {
+                    buf.append(Integer.toHexString((b & 0xFF) + 0x100).substring(1));
+                }
+                filenameHash = buf.toString();


Move out the hash function to adapterutils

andxu · 2017-09-22T08:53:30Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+
+        if (!response.success || respondingTime > RESPONSE_MAX_DELAY_MS) {
+            Map<String, Object> props = new HashMap<>();
+            props.put("respondingTime", respondingTime);


andxu · 2017-09-22T08:53:45Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+        if (!response.success || respondingTime > RESPONSE_MAX_DELAY_MS) {
+            Map<String, Object> props = new HashMap<>();
+            props.put("respondingTime", respondingTime);
+            props.put("requestCommand", requestCommand);


andxu · 2017-09-22T08:58:32Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataStore.java

+        if (th != null) {
+            errorEntry.put("message", th.getMessage());
+            StringWriter sw = new StringWriter();
+            th.printStackTrace(new PrintWriter(sw));


better to close StringWriter PrintWriter

testforstephen · 2017-09-22T09:06:52Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+
+        // bp count
+        if ("setBreakpoints".equals(request.command)) {
+            String filename = request.arguments.get("source").getAsJsonObject().get("path").getAsString();


path could be null. need cover this case.

could use JsonUtils.fromJson(request.arguments, SetBreakpointArguments.class) to get SetBreakpointArguments object.

testforstephen · 2017-09-22T09:20:06Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+
+        props.put("sessionStartAt", String.valueOf(startAt));
+        props.put("sessionStopAt", String.valueOf(stopAt));
+        props.put("commandCount", new Gson().toJson(commandCountMap));


Use JsonUtils.toJson() method instead of new Gson().

testforstephen · 2017-09-22T09:33:23Z

....plugin/src/main/java/com/microsoft/java/debug/plugin/internal/JavaDebuggerServerPlugin.java


 public class JavaDebuggerServerPlugin implements BundleActivator {
    private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME);
+    private static final Logger loggerUserdata = Logger.getLogger(Configuration.USAGE_DATA_LOGGER_NAME);


rename to usageDataLogger

testforstephen · 2017-09-22T09:39:10Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataStore.java

+     * @return List of user data Object.
+     */
+    public List<Object> fetchAll() {
+        List<Object> ret = new ArrayList<>();


Because the result always is parsed to an array on vscode js side, Let fetchAll() return queue.toArray() directly instead of a list.

ansyral · 2017-09-25T03:00:44Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java

+        // bp count
+        if ("setBreakpoints".equals(request.command)) {
+            String fileIdentifier = "unknown file";
+            JsonElement pathElement = request.arguments.get("source").getAsJsonObject().get("path");


request.arguments.get("source").getAsJsonObject().get("path"); [](start = 38, length = 62)

could it possibly raise NullPointerException?

source is supposed not to be null in setBreakpoints, shall we still double check it, or just assume it as NotNull?

ansyral · 2017-09-25T03:08:13Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java

    public void stop() {
+        usageDataSession.reportStop();
        this.terminateSession = true;
+        usageDataSession.submitUsageData();


i'm not sure, but could it possibly be wrapped in sth like a finally block? namely should we report when exception thrown during the session?

ansyral · 2017-09-25T03:32:24Z

a little bit confused, so now if I would like to log something, i should manually specify which logger i'm using?

akaroml · 2017-09-20T10:16:43Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Configuration.java


 public class Configuration {
    public static final String LOGGER_NAME = "java-debug";
+    public static final String USERDATA_LOGGER_NAME = "java-debug-userdata";


java-debug-usage

akaroml · 2017-09-20T10:16:57Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Configuration.java


 public class Configuration {
    public static final String LOGGER_NAME = "java-debug";
+    public static final String USERDATA_LOGGER_NAME = "java-debug-userdata";


USAGE_LOGGER_NAME

akaroml · 2017-09-20T10:17:21Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java

 public class ProtocolServer {
    private static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME);

+    private static final Logger loggerUserdata = Logger.getLogger(Configuration.USERDATA_LOGGER_NAME);


loggerUsage

akaroml · 2017-09-20T10:17:52Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java


+    /* userdataMap Schema:
+     * command: { timestamp: Request JSON String } */
+    private Map<String, Map<Long, String>> userdataMap = new HashMap<>();


userdataMap -> usageData

akaroml · 2017-09-20T10:31:36Z

...soft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ProtocolServer.java

+    private Map<String, Map<Long, String>> userdataMap = new HashMap<>();
+
+    private void recordRequest(Request message) {
+        userdataMap.putIfAbsent(message.command, new HashMap<Long, String>());


Why not just save the return value and use it in the next line instead of getting it.

akaroml · 2017-09-20T10:36:04Z

.../src/main/java/com/microsoft/java/debug/plugin/internal/JavaDebugDelegateCommandHandler.java


 public class JavaDebugDelegateCommandHandler implements IDelegateCommandHandler {

+    public static String FETCH_USER_DATA = "vscode.java.fetchUserData";


FETCH_USER_DATA -> FETCH_USAGE_DATA
vscode.java.fetchUserData -> vscode.java.fetchUsageData

akaroml · 2017-09-20T10:43:21Z

com.microsoft.java.debug.plugin/plugin.xml

            <command id="vscode.java.startDebugSession"/>
            <command id="vscode.java.resolveClasspath"/>
            <command id="vscode.java.buildWorkspace"/>
+            <command id="vscode.java.fetchUserData"/>


fetchUsageData

akaroml · 2017-09-25T03:27:51Z

...debug.plugin/src/main/java/com/microsoft/java/debug/plugin/internal/UsageDataLogHandler.java

+    @Override
+    public void publish(LogRecord record) {
+        if (record.getLevel().intValue() >= thresholdLevel.intValue()) {
+            if (record.getThrown() != null) {


Does this mean a log record does not contain exception or extra data object at the same time?

in Logger, there's no API accepts throwable and extra data at the same time.

akaroml · 2017-09-25T03:51:10Z

com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataStore.java

+    }
+
+    public static UsageDataStore getInstance() {
+        return SingletonHolder.INSTANCE;


If we have multiple sessions going on at the same time, they will share the same session id, which is not expected.

Eskibear requested review from akaroml, andxu and testforstephen September 20, 2017 09:14

msftclas added the cla-not-required label Sep 20, 2017

yaohaizh reviewed Sep 20, 2017

View reviewed changes

Eskibear added 5 commits September 22, 2017 16:31

produce userdata for VSCode to consume

c2ffccd

fix for counting request number

f235179

variable name consistency

c4bbc5e

aggregate data before sending to VSCode

db145d5

repair log

6777dc9

andxu reviewed Sep 22, 2017

View reviewed changes

testforstephen reviewed Sep 22, 2017

View reviewed changes

Eskibear added 2 commits September 23, 2017 01:41

fix according to code review

d1ff793

handle potential NPE

129f598

andxu previously approved these changes Sep 25, 2017

View reviewed changes

ansyral reviewed Sep 25, 2017

View reviewed changes

fix GUID for simultaneous debug sessions

62de460

Eskibear dismissed andxu’s stale review via 62de460 September 25, 2017 07:00

dismiss possible exceptions

9718c20

andxu approved these changes Sep 25, 2017

View reviewed changes

akaroml approved these changes Sep 25, 2017

View reviewed changes

akaroml mentioned this pull request Sep 25, 2017

0.1.0 endgame microsoft/vscode-java-debug#50

Closed

19 tasks

Eskibear merged commit e87577b into microsoft:master Sep 25, 2017


		public class JavaDebugDelegateCommandHandler implements IDelegateCommandHandler {

		public static String FETCH_USER_DATA = "vscode.java.fetchUserData";

Conversation

Eskibear commented Sep 20, 2017

Uh oh!

msftclas commented Sep 20, 2017

Uh oh!

Eskibear commented Sep 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ansyral commented Sep 25, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants